feat(generator/console): add make:controller, make:model commands and the generator attribute #568

gturpin-dev · 2024-10-11T14:35:40Z

This PR is a starting point of creating the make commands for the framework using the internal Generation Component.
Here's the related issue #472

It add a make:controller command

it take a classname as an argument like Test
it will suffix the class with Controller like TestController.php
it will prepend the main autoloaded namespace/path from the composer file to it
it will then glue them with a Controllers directory
it will generate the right namespace PSR-4 compatible
it support adding subfolders directly in the classname arg from the command

So, for example if I have a project with a default autoloaded namespace from composer like :
my-project/app => App
And I run make:controller Admin/Dashboard

It will generate a class named DashboardController at my-project/app/Controllers/Admin/DashboardController.php
And with the following namespace App\Controllers\Admin

Next steps before duplicate this into others make commands are :

Replace the way to retrieve the root path of the final user project
Find a way to test the file is properly generated and at the right location, with the right classname and namespace
Extract the main generation logic to the Generator component
( later ) refactor the file creation logic by the Filesystem component
Handle Dummy replacements in Stubs file
Add MakeControllerCommand specifics params like the route path and the view path

For the 2 first TODOs I'm looking for ideas, if someone have any.

gturpin-dev · 2024-10-11T14:40:08Z

@innocenzi For now, I'm not sure of the purpose of the ClassGenerator but I think the main logic could go there right ?

aidan-casey · 2024-10-11T14:40:50Z

@brendt are we good moving this outside the framework to the CLI project?

innocenzi

I haven't tested this locally, but as I understand it, it will generate a controller in a predefined directory, right?

I don't really like this, personally, and I don't think it follows the philosophy of Tempest, where everything can be anywhere.

My implementation of tempest publish prompts the user every time for the path at which the file would be published. If the file already exists, the user is asked if the file should be replaced. What do you think of this approach?

src/Tempest/Console/src/Commands/Generators/MakeControllerCommand.php

src/Tempest/Generation/tests/ClassGeneratorTest.php

gturpin-dev · 2024-10-11T19:28:30Z

I haven't tested this locally, but as I understand it, it will generate a controller in a predefined directory, right?

I don't really like this, personally, and I don't think it follows the philosophy of Tempest, where everything can be anywhere.

My implementation of tempest publish prompts the user every time for the path at which the file would be published. If the file already exists, the user is asked if the file should be replaced. What do you think of this approach?

You're right, this was the goal.

No problem for me, I can follow your already approved work, I'll check how your command work !
Then, we assume that every make command will always ask for the dist folder, we could imagine to give the main namespace path as an indication in the prompt if the user has no idea.
I'll try to go in this way ( must think about how to generate the right namespace for the file then, maybe show a select for all composer.json autoload or ? )

brendt · 2024-10-12T04:57:29Z

are we good moving this outside the framework to the CLI project?

Will discuss on Discord

coveralls · 2024-10-14T16:54:42Z

Pull Request Test Coverage Report for Build 11331827224

Details

53 of 206 (25.73%) changed or added relevant lines in 8 files are covered.
10 unchanged lines in 5 files lost coverage.
Overall coverage decreased (-1.4%) to 80.589%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/Tempest/Console/src/Stubs/ControllerStub.php	0	2	0.0%
src/Tempest/Support/src/PathHelper.php	29	39	74.36%
src/Tempest/Console/src/Commands/Generators/MakeControllerCommand.php	0	56	0.0%
src/Tempest/Console/src/Commands/PublishCommand.php	0	85	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/Tempest/Generation/src/SimplifiesClassNames.php	1	90.77%
src/Tempest/Auth/src/CreatePermissionsTable.php	2	75.0%
src/Tempest/Auth/src/CreateUserPermissionTable.php	2	77.78%
src/Tempest/Auth/src/CreateUsersTable.php	2	81.82%
src/Tempest/Generation/src/ManipulatesPhpClasses.php	3	69.33%

Totals
Change from base Build 11319238494:	-1.4%
Covered Lines:	6813
Relevant Lines:	8454

💛 - Coveralls

gturpin-dev · 2024-10-14T17:00:35Z

Don't worry about commits, I've pulled the unmerged work of Enzo to refactor the code with its helpers on namespaces and ClassManipulator.

Then I tried to stick as much as possible to the implementation of the publish command.
So actually there is lot of duplicate code between both which is good, because it means we can now abstract those mechanism to create commands that generate files faster.
I'll look into that in the followings days.

gturpin-dev · 2024-10-24T11:29:18Z

As discussed in Discord here https://discord.com/channels/1236153076688359495/1295433932027990027/1298306566335893525
This PR is "ready", we are waiting to see the organization of the "roadmap" for the feature

…asses

…commands

brendt

My main concern is all the hoops we're jumping through to be able to return a StubFileGenerator from a command. There's a lot of complexity that comes with it, which could be avoided if we decide to rework the generator a bit, and instead inject it into the console command.

brendt · 2024-10-29T12:16:41Z

phpstan.neon.dist

-	- phpstan-baseline.neon
-	- vendor/phpat/phpat/extension.neon
-	- vendor/spaze/phpstan-disallowed-calls/extension.neon
+    - phpstan-baseline.neon


Why these changes?

brendt · 2024-10-29T12:17:14Z

composer.json

            "composer phpstan"
        ]
+    },
+    "config": {


Could you remove this? To make sure it doesn't get added again, rebase or merge main into your branch, then run composer up once more.

brendt · 2024-10-29T12:18:35Z

src/Tempest/Console/src/Actions/ExecuteConsoleCommand.php

-                $consoleCommandClass,
-                $inputBuilder->build(),
-            );
+            $handler = ($consoleCommand instanceof GeneratorCommand)


I'm not sure this is the right way to go. I'd be very hesitant making changes within ExecuteConsoleCommand to accomodate for one specific command.

I still need to read through the rest though

brendt · 2024-10-29T12:21:07Z

src/Tempest/Console/src/Commands/Generators/MakeControllerCommand.php

+        $targetPath = $this->promptTargetPath($suggestedPath);
+        $shouldOverride = $this->askForOverride($targetPath);
+
+        return new StubFileGenerator(


We talked about this briefly on Discord: I'm not convinced yet that we need a special kind of console commands that return a special kind of class. I rather just inject the generator into the console command.

Take, for example, a look at how the install command works: it doesn't need to return anything special, but rather passes the responsibility of actually installing a package to the underlying Installer class.

brendt · 2024-10-29T12:21:57Z

src/Tempest/Console/src/Commands/Generators/MakeModelCommand.php

+
+final class MakeModelCommand
+{
+    use HasGeneratorCommand;


This trait should be renamed to IsGeneratorCommand

brendt · 2024-10-29T12:28:51Z

src/Tempest/Generation/tests/ClassGeneratorTest.php

        $this->assertMatchesSnapshot($class->print());
    }

-    #[Test]


Why the removal?

brendt · 2024-10-29T12:35:34Z

src/Tempest/Generation/src/StubFileGenerator.php

+     *     The values are the replacements for the placeholders (e.g. 'App\Models')
+     * @param bool $shouldOverride Whether the generator should override the file if it already exists.
+     */
+    public function __construct(


Is there a reason why this data is injected into the constructor instead of being passed to the generate method? Tempest tries to have all classes injectable, and aims to have a clear separation of data and functionality. When I see a "Stub File Generator", my assumption is that it's a class that I can inject and reuse, while its generate method would require me to pass in context-specific data.

Something like this:

$this->stubGenerator->generate($stub, $target, $replacements, $shouldOverwrite);

This is because atm the generator isn't a service but something like a DTO and the generate method is expected to be run once.
The instance is created and returned by the GeneratorCommand method handler.
In case, if we decide to change the overall approch this could be changed as you mention I guess 😄

brendt · 2024-10-29T12:37:37Z

src/Tempest/Reflection/src/ClassReflector.php

        return $this->reflectionClass->getName();
    }

+    public function getFilePath(): string|false


Let's change this to string. It'll only return false if we're reflecting over internal PHP classes. I'd rather throw an exception in that case:

$path = $this->reflectionClass->getFileName(); if ($path === false) { // throw } return $path;

I'll check this out because I don't remember tweaking on this, so maybe a vestige of the feat/publish

brendt · 2024-10-29T12:38:17Z

src/Tempest/Reflection/src/TypeReflector.php


    public function __construct(
-        private PHPReflector|PHPReflectionType|string $reflector,
+        private PHPReflector|PHPReflectionType|string|null $reflector,


Out of curiosity: when does this happen?

It happen to me weeks ago but if I remember correctly, an instance of the TypeReflector were created dynamically and something null passed in param so it throws an exception.
But giving "null" works.

brendt · 2024-10-29T12:39:02Z

src/Tempest/Support/src/PathHelper.php

+
 final readonly class PathHelper
 {
+    public static function root(string ...$paths): string


I don't think we need this one anymore. Use root_path() instead

You're right, i'll correct this ASAP

brendt · 2024-10-31T12:22:28Z

Let's close this one in favor of #647

innocenzi reviewed Oct 11, 2024

View reviewed changes

gturpin-dev changed the title ~~feat(generator/console): add make:controller command~~ feat(generator/console): add make:controller command Oct 22, 2024

gturpin-dev changed the title ~~feat(generator/console): add make:controller command~~ feat(generator/console): add make:controller and make:model commands with the generator command attribute Oct 22, 2024

gturpin-dev changed the title ~~feat(generator/console): add make:controller and make:model commands with the generator command attribute~~ feat(generator/console): add make:controller, make:model commands and the generator attribute Oct 22, 2024

innocenzi and others added 19 commits October 28, 2024 17:03

feat(generation): add ClassGenerator

5f09cad

✨ Add NamespaceHelper class

8994764

✨ Working version of the make:controller command

ee44096

🎨 Format code

76063f4

🔥 Comment breaking test from unmerged branch

1023fdc

feat: add ability to publish vendor files

a487fe2

fix(path-helper): pass paths to root

8bb0f99

feat: add getFilePath to ClassReflector

d9ee3d9

refactor: use ClassReflector

089f694

feat(publish): use Composer util to generate default publish paths

912f23e

feat(path-helper): add toNamespace and toRegisteredNamespace

7052427

feat: mark migrations as publishable

3953165

refactor: rename publishFiles to publishClasses

c8737ad

feat(publish): remove attributes and update namespace of published cl…

1d7a424

…asses

test: fix assumption about migrations being registered

a26ae6a

style: apply fixes from rector

8ad5b76

feat: support publishing files

03ad829

♻️ Refactor make:controller using ToNamespace helper from PathHelper

d81a302

🔥 Delete useless NamespaceHelper duplicates

b701dee

gturpin-dev added 18 commits October 28, 2024 17:08

🎨 Prepare generator command abstraction skeleton

ecdfecb

🐛 Allow TypeReflector to get null value

6f94a6d

✨ Implement the GeneratorCommand and the Discovery to discover those …

8a4dc32

…commands

♻️ Refactor the GeneratorCommand to extends from ConsoleCommand

c755523

✨ Implement the trait's methods

4594bb6

✨ Handle specific handlers for generator commands

872757c

✨ Make the StubFileGenerator logic

9e2127a

✨ conditionally generate model stub

fed3dae

✨ Make Generator commands with multiple handler and generators works

5e5aebe

🔥 Revert unwanted changes for now

56e5b7d

♻️ Refactor factory to GeneratorCommand specific method

e6d78b8

🎨 Coding styles

cf9de26

🚨 Fix phpstan

af2af0b

🐛 Fix unused variable issue

900a57f

♻️ Refactor make:controller command to use the new generator api

925aa11

🐛 Fix basics command issue

c396bff

🐛 Fix issues in make:controller command and dummy replacements

6f45e99

🎨 Coding styles

39b0ca9

gturpin-dev force-pushed the feat/console/generator-commands branch from 7fe30ac to 39b0ca9 Compare October 28, 2024 16:11

gturpin-dev added 5 commits October 28, 2024 17:16

✨ Allow php-http/discovery

e3e9ce0

🐛 Fix phpstan config issue

a3e8b84

🔥 Remove feat/publish dependency

82eabdb

🐛 Fix issue

3c94f12

🚨 Run Rector

2634717

gturpin-dev marked this pull request as ready for review October 28, 2024 16:53

brendt requested changes Oct 29, 2024

View reviewed changes

gturpin-dev mentioned this pull request Oct 30, 2024

feat(console): add make:controller and make:model commands #647

Merged

brendt closed this Oct 31, 2024

gturpin-dev deleted the feat/console/generator-commands branch November 12, 2024 12:08

Uh oh!

feat(generator/console): add make:controller, make:model commands and the generator attribute #568

feat(generator/console): add make:controller, make:model commands and the generator attribute #568

Uh oh!

Conversation

gturpin-dev commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gturpin-dev commented Oct 11, 2024

Uh oh!

aidan-casey commented Oct 11, 2024

Uh oh!

innocenzi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gturpin-dev commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brendt commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Oct 14, 2024

Pull Request Test Coverage Report for Build 11331827224

Details

💛 - Coveralls

Uh oh!

gturpin-dev commented Oct 14, 2024

Uh oh!

gturpin-dev commented Oct 24, 2024

Uh oh!

brendt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brendt commented Oct 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gturpin-dev commented Oct 11, 2024 •

edited

Loading

gturpin-dev commented Oct 11, 2024 •

edited

Loading

brendt commented Oct 12, 2024 •

edited

Loading